Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Mesh Skinning. Attempt #3 #4238

Closed
wants to merge 33 commits into from

Conversation

james7132
Copy link
Member

@james7132 james7132 commented Mar 17, 2022

Objective

Load skeletal weights and indices from GLTF files. Animate meshes.

Solution

  • Load skeletal weights and indices from GLTF files.
  • Added SkinnedMesh component and SkinnedMeshInverseBindPose asset
  • Added extract_skinned_meshes to extract joint matrices.
  • Added queue phase systems for enqueuing the buffer writes.

Some notes:

  • This ports part of # Implement mesh skinning #2359 to the current main.
  • This generates new BufferVecs and bind groups every frame. The expectation here is that the number of Query::get calls during extract is probably going to be the stronger bottleneck, with up to 256 calls per skinned mesh. Until that is optimized, caching buffers and bind groups is probably a non-concern.
  • Unfortunately, due to the uniform size requirements, this means a 16KB buffer is allocated for every skinned mesh every frame. There's probably a few ways to get around this, but most of them require either compute shaders or storage buffers, which are both incompatible with WebGL2.

Crediting

This isn't something that I did alone and has been worked on by @lassade and @Looooong and multiple others before this PR was made.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Mar 17, 2022
@james7132 james7132 added A-Assets Load files from disk to use for things like images, models, and sounds A-Animation Make things move and change over time labels Mar 17, 2022
@james7132 james7132 marked this pull request as draft March 17, 2022 10:22
@james7132 james7132 added C-Enhancement A new feature and removed S-Needs-Triage This issue needs to be labelled labels Mar 17, 2022
@james7132 james7132 changed the title Load mesh skinning data from GLTF files Mesh Skinning. Attempt #3 Mar 17, 2022
@james7132 james7132 marked this pull request as ready for review March 20, 2022 04:19
@james7132 james7132 added this to the Bevy 0.7 milestone Mar 21, 2022
@james7132 james7132 mentioned this pull request Mar 21, 2022
14 tasks
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just finished my first pass. Generally things are looking good. I think the SkinnedMeshJoints allocation patterns are the biggest thing that needs revisiting.

crates/bevy_pbr/src/render/light.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/mesh/mesh/mod.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_pbr/src/render/mesh.rs Outdated Show resolved Hide resolved
crates/bevy_pbr/src/render/mesh.rs Outdated Show resolved Hide resolved
},
count: None,
}],
label: Some("mesh_layout"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
label: Some("mesh_layout"),
label: Some("joint_buffer_layout"),

Nit: distinct name for debugging.

@cart
Copy link
Member

cart commented Mar 28, 2022

I consolidated the joint binding into the Mesh bind group, which I think is the right path forward, given how limited bind group slots are in practice (4 is what we can rely on).

I think this is good to go. I'll wait for feedback on the merged bind group from at least one other person involved in this before merging though.

@james7132
Copy link
Member Author

How does WGSL/wgpu handle bindings that go unused? I was originally under the impression that the two had to be 1:1 Any skinned mesh on screen would add an extra binding to all drawn meshes.

@cart
Copy link
Member

cart commented Mar 29, 2022

How does WGSL/wgpu handle bindings that go unused? I was originally under the impression that the two had to be 1:1 Any skinned mesh on screen would add an extra binding to all drawn meshes.

Bindings must be used / bound. Note that in my change there are two "flavors" of the bind group: one with skinning and one without. The proper bind group is selected based on whether or not skinning is enabled. It is always a "perfect fit".

@james7132
Copy link
Member Author

While I think this will work for this particular case, I'm not sure how well this will go when we need expanded support (i.e. morph targets), as we'd have combinatorially many bind groups. I guess we'll cross that bridge when we get there.

@cart
Copy link
Member

cart commented Mar 29, 2022

While I think this will work for this particular case, I'm not sure how well this will go when we need expanded support (i.e. morph targets), as we'd have combinatorially many bind groups. I guess we'll cross that bridge when we get there

Depends on the approach used. If we use dynamic uniforms for morph targets, we can still have a single bind group. Happy to revisit when we implement morph targets though.

@cart
Copy link
Member

cart commented Mar 29, 2022

bors r+

bors bot pushed a commit that referenced this pull request Mar 29, 2022
# Objective
Load skeletal weights and indices from GLTF files. Animate meshes.

## Solution
 - Load skeletal weights and indices from GLTF files.
 - Added `SkinnedMesh` component and ` SkinnedMeshInverseBindPose` asset
 - Added `extract_skinned_meshes` to extract joint matrices.
 - Added queue phase systems for enqueuing the buffer writes.

Some notes:

 -  This ports part of # #2359 to the current main.
 -  This generates new `BufferVec`s and bind groups every frame. The expectation here is that the number of `Query::get` calls during extract is probably going to be the stronger bottleneck, with up to 256 calls per skinned mesh. Until that is optimized, caching buffers and bind groups is probably a non-concern.
 - Unfortunately, due to the uniform size requirements, this means a 16KB buffer is allocated for every skinned mesh every frame. There's probably a few ways to get around this, but most of them require either compute shaders or storage buffers, which are both incompatible with WebGL2.

Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: James Liu <contact@jamessliu.com>
@bors bors bot changed the title Mesh Skinning. Attempt #3 [Merged by Bors] - Mesh Skinning. Attempt #3 Mar 29, 2022
@bors bors bot closed this Mar 29, 2022
@adsick
Copy link
Contributor

adsick commented Apr 1, 2022

there is some Z-fighting in 'custom_skinned_mesh' example, not like a major issue but it's better not to see things like that)

aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
# Objective
Load skeletal weights and indices from GLTF files. Animate meshes.

## Solution
 - Load skeletal weights and indices from GLTF files.
 - Added `SkinnedMesh` component and ` SkinnedMeshInverseBindPose` asset
 - Added `extract_skinned_meshes` to extract joint matrices.
 - Added queue phase systems for enqueuing the buffer writes.

Some notes:

 -  This ports part of # bevyengine#2359 to the current main.
 -  This generates new `BufferVec`s and bind groups every frame. The expectation here is that the number of `Query::get` calls during extract is probably going to be the stronger bottleneck, with up to 256 calls per skinned mesh. Until that is optimized, caching buffers and bind groups is probably a non-concern.
 - Unfortunately, due to the uniform size requirements, this means a 16KB buffer is allocated for every skinned mesh every frame. There's probably a few ways to get around this, but most of them require either compute shaders or storage buffers, which are both incompatible with WebGL2.

Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: James Liu <contact@jamessliu.com>
@james7132 james7132 deleted the skeletal-animation branch June 22, 2022 08:25
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective
Load skeletal weights and indices from GLTF files. Animate meshes.

## Solution
 - Load skeletal weights and indices from GLTF files.
 - Added `SkinnedMesh` component and ` SkinnedMeshInverseBindPose` asset
 - Added `extract_skinned_meshes` to extract joint matrices.
 - Added queue phase systems for enqueuing the buffer writes.

Some notes:

 -  This ports part of # bevyengine#2359 to the current main.
 -  This generates new `BufferVec`s and bind groups every frame. The expectation here is that the number of `Query::get` calls during extract is probably going to be the stronger bottleneck, with up to 256 calls per skinned mesh. Until that is optimized, caching buffers and bind groups is probably a non-concern.
 - Unfortunately, due to the uniform size requirements, this means a 16KB buffer is allocated for every skinned mesh every frame. There's probably a few ways to get around this, but most of them require either compute shaders or storage buffers, which are both incompatible with WebGL2.

Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: James Liu <contact@jamessliu.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time A-Assets Load files from disk to use for things like images, models, and sounds C-Enhancement A new feature
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants